Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Perf-Testing: Cleanups for perf regression caused by initial implementation of RFC 3191 #96795

Closed

Conversation

ridwanabdillahi
Copy link
Contributor

@ridwanabdillahi ridwanabdillahi commented May 6, 2022

This is currently a Draft and not ready for review.

PR #91779 implemented initial support for RFC 3191 which caused a noticeable perf regression tracked by #96786. Implementing a few different changes to drive down the perf regression such as only querying for the #[debugger_visualizer] attribute when targeting msvc.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 6, 2022
@rust-highfive
Copy link
Collaborator

r? @jackh726

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 6, 2022
@ridwanabdillahi
Copy link
Contributor Author

r? @wesleywiser

@rust-highfive rust-highfive assigned wesleywiser and unassigned jackh726 May 6, 2022
@davidtwco
Copy link
Member

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 7, 2022
@bors
Copy link
Contributor

bors commented May 7, 2022

⌛ Trying commit da7556400d03631b132ba06450724220ebb8f745 with merge c82d40f772049273ff47b19f47a22915e324459f...

@bors
Copy link
Contributor

bors commented May 7, 2022

☀️ Try build successful - checks-actions
Build commit: c82d40f772049273ff47b19f47a22915e324459f (c82d40f772049273ff47b19f47a22915e324459f)

@rust-timer
Copy link
Collaborator

Queued c82d40f772049273ff47b19f47a22915e324459f with parent 4799baa, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (c82d40f772049273ff47b19f47a22915e324459f): comparison url.

Summary: This benchmark run did not return any relevant results.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 7, 2022
@compiler-errors
Copy link
Member

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 11, 2022
@bors
Copy link
Contributor

bors commented May 11, 2022

⌛ Trying commit 5797c9158ae0f27c3f1f8af1b91fb192687b039f with merge f255f5f55d944ee5b4eb745966edac0924631545...

@bors
Copy link
Contributor

bors commented May 11, 2022

☀️ Try build successful - checks-actions
Build commit: f255f5f55d944ee5b4eb745966edac0924631545 (f255f5f55d944ee5b4eb745966edac0924631545)

@rust-timer
Copy link
Collaborator

Queued f255f5f55d944ee5b4eb745966edac0924631545 with parent 08b4f1b, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (f255f5f55d944ee5b4eb745966edac0924631545): comparison url.

Summary: This benchmark run did not return any relevant results.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

@rust-timer
Copy link
Collaborator

Queued 5e04e4593df4d3137047ada6ef58386fa6facb4a with parent f1f721e, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (5e04e4593df4d3137047ada6ef58386fa6facb4a): comparison url.

Summary: This benchmark run did not return any relevant results.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 14, 2022
@wesleywiser
Copy link
Member

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 16, 2022
@bors
Copy link
Contributor

bors commented May 16, 2022

⌛ Trying commit 3c87c4d with merge 56c049b2e9a3a9185825ecfeeec6dbddee17bc66...

@bors
Copy link
Contributor

bors commented May 16, 2022

☀️ Try build successful - checks-actions
Build commit: 56c049b2e9a3a9185825ecfeeec6dbddee17bc66 (56c049b2e9a3a9185825ecfeeec6dbddee17bc66)

@rust-timer
Copy link
Collaborator

Queued 56c049b2e9a3a9185825ecfeeec6dbddee17bc66 with parent c52b9c1, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (56c049b2e9a3a9185825ecfeeec6dbddee17bc66): comparison url.

Summary:

  • Primary benchmarks: 😿 relevant regression found
  • Secondary benchmarks: no relevant changes found
Regressions 😿
(primary)
Regressions 😿
(secondary)
Improvements 🎉
(primary)
Improvements 🎉
(secondary)
All 😿 🎉
(primary)
count1 1 0 0 0 1
mean2 0.5% N/A N/A N/A 0.5%
max 0.5% N/A N/A N/A 0.5%

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

Footnotes

  1. number of relevant changes

  2. the arithmetic mean of the percent change

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 16, 2022
@lqd
Copy link
Member

lqd commented May 17, 2022

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 17, 2022
@bors
Copy link
Contributor

bors commented May 17, 2022

⌛ Trying commit 485e105 with merge 1aa8aeb6901dae58833b658df1c0aae2384fc792...

@bors
Copy link
Contributor

bors commented May 18, 2022

☀️ Try build successful - checks-actions
Build commit: 1aa8aeb6901dae58833b658df1c0aae2384fc792 (1aa8aeb6901dae58833b658df1c0aae2384fc792)

@rust-timer
Copy link
Collaborator

Queued 1aa8aeb6901dae58833b658df1c0aae2384fc792 with parent 4c5f6e6, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (1aa8aeb6901dae58833b658df1c0aae2384fc792): comparison url.

Summary:

  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: 😿 relevant regression found
Regressions 😿
(primary)
Regressions 😿
(secondary)
Improvements 🎉
(primary)
Improvements 🎉
(secondary)
All 😿 🎉
(primary)
count1 0 1 0 0 0
mean2 N/A 2.0% N/A N/A N/A
max N/A 2.0% N/A N/A N/A

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

Footnotes

  1. number of relevant changes

  2. the arithmetic mean of the percent change

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 18, 2022
@ridwanabdillahi
Copy link
Contributor Author

The tracking issue for that this PR was tracking has been closed after posting results of these experiments.

@ridwanabdillahi ridwanabdillahi deleted the perf-cleanup branch May 27, 2022 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.